fix: guard wait() against transient FAILED during saga startup#203
Merged
Conversation
wait()'s poll safety-net could see FAILED before the saga transitioned the run to COMPENSATING. When the tracker event has not yet fired, defer to the event instead of returning the possibly-transient terminal state. Also retry _mark_run_compensating on transient "database is locked" errors so SQLite contention does not silently leave the run stuck in FAILED.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR hardens the saga compensation flow by addressing two sides of the state transition. The orchestrator now retries transient database-lock errors when marking a run as Compensating, and the wait loop defers returning terminal states when a compensation transition is in flight, waiting for the tracker event instead of returning a potentially-stale status. ChangesSaga Compensation Transition Reliability
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The previous fix (#201) moved
_mark_run_compensatingearlier instart_compensationto narrow the FAILED→COMPENSATING race window, but Python overhead betweenmark_workflow_node_result(Rust sets FAILED) and the Python-side transition still left a gap the 100ms poll could catch on slow Windows CI.This PR eliminates the race entirely with two changes:
wait()defers to the tracker event on transient terminal states. When the polled state isFAILEDorCOMPLETED_WITH_FAILURESand the tracker's waiter event hasn't been set yet,wait()defers to the event instead of returning immediately. The event is set only after the saga decision is made — either the saga finishes (COMPENSATED/COMPENSATION_FAILED) or no saga starts (FAILED is emitted as terminal). This closes the race regardless of how long the Python overhead takes._mark_run_compensatingretries on "database is locked". Transient SQLite write contention could silently leave the run in FAILED. The method now retries up to 5 times with backoff, matching the existing pattern in_dispatch_single.Test plan
Summary by CodeRabbit